Fix redundant/broken intradoc links and minor cleanup #716
Conversation
|
Thanks a lot for this work, your suggestions seem quite good. 👍 I honestly don't have any strong feelings about the precise documentation style we use, but I agree that having a consistent one would absolutely be a good thing. Do you have any thoughts on that @ids1024 ? |
|
Thanks @elinorbgr! Since proposing that list I've made a few picks already, bit the bullet, and pushed the changes to consistenize the docs. Only removing There were a few non-doc |
|
Since I have rebased, I should probably check if there are any new docs merged since that time that need touch-ups. And maybe set the "style guideline" in stone somewhere? |
|
It wasn't too much but there's one point of confusion that I outlined here: https://github.com/Smithay/wayland-rs/pull/680/files#r1592423992 By at least pushing an intradoc link from |
|
Looks like |
|
Presumably CI will pass if this is rebased. |
…d parenthesis Keeping the link target around is redundant as `rustdoc` already infers this from the link name, after removing the backticks. It uses the call-parenthesis to enforce that the link target is indeed an `fn` (and not some other item).
…y_ptr()` This moved for the server but not the client `Backend`, and because of not being a link nor validated in the CI this broken reference wasn't noticed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #716 +/- ##
==========================================
+ Coverage 73.11% 75.93% +2.81%
==========================================
Files 45 46 +1
Lines 8183 8197 +14
==========================================
+ Hits 5983 6224 +241
+ Misses 2200 1973 -227
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Looks like a local Rust 1.78 run shows quite a few more issues now 🎉 EDIT: The CI here doesn't see them, but I've fixed it anyway. Fortunately most of the dead links locally were caused by the missing |
After spotting a missing closing parenthesis in
[`EventQueue::blocking_dispatch()`](EventQueue::blocking_dispatch, leading to the link not rendering correctly, I set out on a course to delete these unnecessary link targets as[`EventQueue::blocking_dispatch()`]on its own already translates to the expected doc link, without being verbose. At the same time this PR includes a smattering of improvements, such as fixing missing links and updating mentions of functions that have been renamed or moved.Finally, this PR sets up an extra CI step to make sure documentation doesn't have any build errors going forward.
I don't think this PR should be merged as-is, before we set up a little style guide for docs and finish the remainder. There's a bit of a mix currently but nothing we can't fix, so I'd like to ask the maintainers for their preference/agreement:
[]()links to[][], as the former converts into a nonsense URL sometimes when the target is not a valid item and there's no matching/// [bar]: ...label target in scope, whereas the latter fails the build appropriately (as described by me in On wasm, provide intradoc-link forspawn()function inEventLoopdocs rust-windowing/winit#3178 (comment));()in the name to avoid ambiguity;[`foo()`][Self::foo()]) should use parenthesis in the target as well, again to avoid ambiguity;Self, should be named as:[`FullStructName::func()`];[`func()`][FullStructName::func()];[`func()`][Self::func()];cratein them (like[`crate::event_created_child!()`]) should be avoided and instead use an explicit link target.Thanks for considering this! The documentation is already great and I hope this makes it more consistent (and less likely to reference (re)moved items) going forward!